[MDS-6691] Save more data about MS users #3724
Conversation
…e data. Add more fields to MS user table, update model. Add versioning, fix issues with circular dependencies so it works. Update a field name. TS-ify the user list page
| if not bceid_username: | ||
| raise AssertionError('Identifier is not provided.') | ||
| return email_or_username | ||
| if not bceid_username.endswith('@bceid'): |
There was a problem hiding this comment.
added a BE error when there's a bad BCeID passed in from the FE. I didn't add validation on the FE, figuring that the form will be revamped.
|
|
||
|
|
||
| class User(SoftDeleteMixin, AuditMixin, Base): | ||
| class User(HistoryMixin, SoftDeleteMixin, AuditMixin, Base): |
There was a problem hiding this comment.
was mostly experimenting when I was trying to figure out why versioning wasn't working! Anyway, this adds a history attribute that'll have nicely parsed data, so I left it in.
|
|
||
| user = User.create_or_update_user(**user_data) | ||
| if is_minespace_user(): | ||
| # bceid given/family name may be combined |
There was a problem hiding this comment.
my particular token data has the following:
given_name: "Tara Epp"
display_name: "Tara Epp"
family_name: ""
It's a pretty small sample size, but prod data can be checked later on to see how the parsing is going and possibly modify it from there. The display_name is what's used by the FE.
|
|
||
|
|
||
| def get_mine_access(): | ||
| from .api.users.minespace.models.minespace_user import MinespaceUser |
There was a problem hiding this comment.
the imports from MinespaceUser before the app is initialized is what caused versioning to not work, so now they're loaded in the functions. There is a BE test added to catch the versioning breaking again.
| if given_name == display_name and family_name == "": | ||
| name = display_name.split() | ||
| given_name = name[0] if len(name) > 0 else "" | ||
| family_name = name[1] if len(name) > 1 else "" |
There was a problem hiding this comment.
I'm thinking we might just want to accept empty family name if not provided in the bceid mapping, as in some cases this wouldn't be correct (e.g. in my case!)
Given Name: Simen Fivelstad
Family Name: Smaaberg
There was a problem hiding this comment.
Hmm yeah. Yours would parse as given (the if would be false) but we may as well see how weird the data is before trying to clean it. Using display_name on the FE prevents the weirdness from showing up anywhere visible to any users.
| } | ||
|
|
||
| user = MinespaceUser.update_from_token_data(**user_data) | ||
| print(user) |
| if request.args.get('email_or_username'): | ||
| ms_users = [MinespaceUser.find_by_email(request.args.get('email_or_username'))] | ||
| if request.args.get('bceid_username'): | ||
| ms_users = [MinespaceUser.find_by_username(request.args.get('bceid_username'))] |
There was a problem hiding this comment.
Hmm something feels a bit odd with this logic here reading this whole function. I think we should probably also filter this by the mine_guid you pass in if you're a minespace proponent?
There was a problem hiding this comment.
this is some 5 year old code, which, based on the FE code, I think is actually just dead. There's no calls to the API matching this resource where we do a GET with anything other than mine_guid. I'll take it out.
services/core-api/app/api/users/minespace/models/minespace_user.py
Outdated
Show resolved
Hide resolved
|
|
|
|
Objective
MDS-6691
Why are you making this change? Provide a short explanation and/or screenshots
The User Access table (same on Core):
Admin page on Core (got bonus columns and quick TS update):
